-
-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Fix issue #11189 - Implement a caching solution with local storage for citation relations #11845
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix issue #11189 - Implement a caching solution with local storage for citation relations #11845
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your code currently does not meet JabRef's code guidelines.
We use Checkstyle to identify issues.
The tool reviewdog already placed comments on GitHub to indicate the places. See the tab "Files" in you PR.
Please carefully follow the setup guide for the codestyle.
Afterwards, please run checkstyle locally and fix the issues.
You can check review dog's comments at the tab "Files changed" of your pull request.
9fe8522 to
cbe9e96
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your code currently does not meet JabRef's code guidelines.
We use Checkstyle to identify issues.
The tool reviewdog already placed comments on GitHub to indicate the places. See the tab "Files" in you PR.
Please carefully follow the setup guide for the codestyle.
Afterwards, please run checkstyle locally and fix the issues.
You can check review dog's comments at the tab "Files changed" of your pull request.
cbe9e96 to
8231340
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your code currently does not meet JabRef's code guidelines.
We use OpenRewrite to ensure "modern" Java coding practices.
The issues found can be automatically fixed.
Please execute the gradle task rewriteRun, check the results, commit, and push.
You can check the detailed error output by navigating to your pull request, selecting the tab "Checks", section "Tests" (on the left), subsection "OpenRewrite".
8231340 to
33967c2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your code currently does not meet JabRef's code guidelines.
We use OpenRewrite to ensure "modern" Java coding practices.
The issues found can be automatically fixed.
Please execute the gradle task rewriteRun, check the results, commit, and push.
You can check the detailed error output by navigating to your pull request, selecting the tab "Checks", section "Tests" (on the left), subsection "OpenRewrite".
33967c2 to
592d4d7
Compare
d94f4d3 to
3155242
Compare
alexandre-cremieux
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add code explanations to the PR
src/main/java/org/jabref/gui/entryeditor/citationrelationtab/BibEntryRelationsCache.java
Show resolved
Hide resolved
src/main/java/org/jabref/gui/entryeditor/citationrelationtab/BibEntryRelationsRepository.java
Show resolved
Hide resolved
src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java
Show resolved
Hide resolved
src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java
Show resolved
Hide resolved
src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java
Show resolved
Hide resolved
src/main/java/org/jabref/logic/citation/repository/LRUBibEntryRelationsCache.java
Outdated
Show resolved
Hide resolved
src/main/java/org/jabref/logic/citation/repository/LRUBibEntryRelationsCache.java
Outdated
Show resolved
Hide resolved
src/main/java/org/jabref/logic/citation/repository/LRUBibEntryRelationsRepository.java
Outdated
Show resolved
Hide resolved
src/main/java/org/jabref/logic/importer/fetcher/CitationFetcher.java
Outdated
Show resolved
Hide resolved
src/main/java/org/jabref/logic/citation/service/SearchCitationsRelationsService.java
Outdated
Show resolved
Hide resolved
|
Please no force push if not needed. All commits will be squashed when merged |
* Move repository, cache, and fetcher to logic package * Move citations model to model/citations/semanticscholar package
* Introduce service layer * Rename LRU cache implementation * Add tests helpers for repository
* Move logic from repository to service * Refactor repositories * Update tab configuration
3155242 to
18db75e
Compare
Sorry, just re-based main branch locally. |
…lation-tab-logic # Conflicts: # src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java
koppor
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general looks good. Some minor comments.
Sorry for delay. Please go ahead with everything.
src/main/java/org/jabref/logic/citation/service/SearchCitationsRelationsService.java
Outdated
Show resolved
Hide resolved
src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java
Outdated
Show resolved
Hide resolved
src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java
Show resolved
Hide resolved
src/test/java/org/jabref/logic/citation/service/SearchCitationsRelationsServiceTest.java
Outdated
Show resolved
Hide resolved
|
Small other comments - IntelliJ proposed to extract a method in the tests - maybe you can also include that. |
|
@alexandre-cremieux Please pull before you continue working on it - I merged |
src/main/java/org/jabref/logic/citation/repository/LRUBibEntryRelationsRepository.java
Outdated
Show resolved
Hide resolved
src/main/java/org/jabref/logic/citation/service/SearchCitationsRelationsService.java
Outdated
Show resolved
Hide resolved
src/main/java/org/jabref/logic/citation/service/SearchCitationsRelationsService.java
Outdated
Show resolved
Hide resolved
Thanks for the review and the merge. I will resume the work on this branch and apply the changes. |
|
@alexandre-cremieux Sorry for the merge conflicts - can you handle them? I was always happy with IntelliJ's "resolve merge conflicts" dialog. Hope, it works in this case, too. |
Hello @koppor . Seems that we have new conflicts to resolve to be able to merge main. But I will do that when the feature will be fully developed. Was quite busy last month, I resumed the work this week. PR comments were addressed. |
I've done both - if tests are slower on the users end, we need to work on this again. |
Co-authored-by: Oliver Kopp <[email protected]>
…-00-refactor-citation-relation-tab-logic' into fork/alexandre-cremieux/fix-issue-11189-part-00-refactor-citation-relation-tab-logic
jabgui/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java
Show resolved
Hide resolved
Co-authored-by: Oliver Kopp <[email protected]>
jablib/src/main/java/org/jabref/logic/citation/SearchCitationsRelationsService.java
Show resolved
Hide resolved
|
@trag-bot didn't find any issues in the code! ✅✨ |
High level description
This contributions aims to provide a local storage solution for citations relations using MVStore. Please see #11189 for more information.
Fixes #11189
Implementation details
No new dependency added.
Caching strategy
The caching logic of the citations/references is now fully handled by h2.mvstore.MVStore. The store is responsible of the in memory caching and the offline storage logic.
This aims to avoid JabRef to access the Internet if the user already searched for citations/references related to one of its bib entry.
The cache/local store is common to all the libraries managed by the JabRef instance.
Search caching high level logic
Serialization
MVStoreDAO uses the canonical representation to serialize a BibEntry and the
BibtexParserto deserialize it.Configuration
Citations Relationstab using the JabRef IoC providercitationsfolder under JabRef's app directorySetting translation
The the local storage TTL setting message has been traduced in:
Refactoring
This contributions simplifies the citations/references fetching and caching logic by introducing two layers:
This should help to make this feature more extendable without modifying orchestration logic following open/close principle.
Screen shots
Mandatory checks
CHANGELOG.mddescribed in a way that is understandable for the average user (if applicable)